Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add nameserver 1.1.1.1 to conformance-runtime test LVM #29455

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Nov 28, 2023

With the removal of the property dns-resolver to set a nameserver via the little-vm-helper GitHub action, conformance-runtime tests are failing quite often with the following error.

dial tcp: lookup quay.io on 127.0.0.53:53: read udp 127.0.0.1:40553->127.0.0.53:53: read: connection refused

The assumption is that the local DNS resolver isn't ready at that time when pulling the cilium docker plugin image (rather early after startup).

Therefore, this commit temporarily re-adds the nameserver 1.1.1.1 manually in the runtime test - at the beginning of /etc/resolv.conf - and therefore prioritized.

See: cilium/little-vm-helper#118

@mhofstetter mhofstetter added kind/bug This is a bug in the Cilium logic. area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Nov 28, 2023
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/runtime-dns-fix branch 2 times, most recently from bd1bdb0 to b1d7bc0 Compare November 29, 2023 07:00
@mhofstetter
Copy link
Member Author

/ci-runtime

With the removal of setting a nameserver via the little-vm-helper
GitHub action, conformance-runtime tests are failing quite often
with the following error.

```
dial tcp: lookup quay.io on 127.0.0.53:53: read udp 127.0.0.1:40553->127.0.0.53:53: read: connection refused
```

The assumption is that the local DNS resolver isn't ready at that time when pulling the image.

Therefore, this way temporarily re-adds the nameserver `1.1.1.1` manually.

See: cilium/little-vm-helper#118

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
The property dns-resolver has been removed from the little-vm-helper
GitHub action.

Therefore, this commit removes the usage of it in the cilium repository.

See: cilium/little-vm-helper#118

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter requested a review from brb November 29, 2023 10:58
@mhofstetter mhofstetter marked this pull request as ready for review November 29, 2023 10:58
@mhofstetter mhofstetter requested review from a team as code owners November 29, 2023 10:58
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find! Shall we bring back the custom resolver setting to the LVH action?

Copy link
Contributor

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 29, 2023
@tklauser tklauser added this pull request to the merge queue Nov 29, 2023
@mhofstetter
Copy link
Member Author

Good find! Shall we bring back the custom resolver setting to the LVH action?

I don't think it's necessary. All other cases that use the LVH are working fine.

@brb I think it would be better to check the status of the resolver in the VM before allowing commands via the github actions API. e.g. solving in the LVH. WDYT?

Merged via the queue into main with commit a2694fc Nov 29, 2023
223 checks passed
@tklauser tklauser deleted the pr/mhofstetter/runtime-dns-fix branch November 29, 2023 12:32
@brb
Copy link
Member

brb commented Nov 29, 2023

I think it would be better to check the status of the resolver in the VM before allowing commands via the github actions API. e.g. solving in the LVH. WDYT?

SGTM.

@tklauser tklauser added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Dec 8, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.5 Dec 8, 2023
@tklauser tklauser added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Dec 8, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.10 Dec 8, 2023
@tklauser
Copy link
Member

tklauser commented Dec 8, 2023

FYI, I've marked this PR to be backported to v1.14 and v1.13 where we've also updated the little-vm-helper action and the workflows using it now show warnings about unexpected dns-resolver input, see e.g. https://github.com/cilium/cilium/actions/runs/7140854863

@nebril nebril added this to Needs backport from main in 1.14.6 Dec 11, 2023
@nebril nebril removed this from Needs backport from main in 1.14.5 Dec 11, 2023
@nebril nebril added this to Needs backport from main in 1.13.11 Dec 11, 2023
@nebril nebril removed this from Needs backport from main in 1.13.10 Dec 11, 2023
@giorio94 giorio94 mentioned this pull request Dec 13, 2023
4 tasks
@giorio94 giorio94 added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Dec 13, 2023
@giorio94 giorio94 mentioned this pull request Dec 13, 2023
10 tasks
@giorio94 giorio94 added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Dec 13, 2023
@github-actions github-actions bot added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Dec 13, 2023
@gentoo-root gentoo-root moved this from Needs backport from main to Backport done to v1.13 in 1.13.11 Jan 17, 2024
@gentoo-root gentoo-root moved this from Needs backport from main to Backport done to v1.14 in 1.14.6 Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.13.11
Backport done to v1.13
1.14.6
Backport done to v1.14
Status: Released
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

5 participants